perf(formatter): Allocation free interned#7443
Closed
MichaReiser wants to merge 1 commit intobest-fitting-reduce-allocationsfrom
Closed
perf(formatter): Allocation free interned#7443MichaReiser wants to merge 1 commit intobest-fitting-reduce-allocationsfrom
MichaReiser wants to merge 1 commit intobest-fitting-reduce-allocationsfrom
Conversation
Member
Author
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
e462ac3 to
673eeda
Compare
CodSpeed Performance ReportMerging #7443 will degrade performances by 2.45%Comparing Summary
Benchmarks breakdown
|
07d3611 to
4ae041f
Compare
1b77a35 to
958c8a0
Compare
958c8a0 to
a2f8e71
Compare
3283f0f to
a638734
Compare
ddd5e34 to
2843fc0
Compare
a638734 to
e112b02
Compare
Member
Author
|
We may want to bring this back when working on other best fitting layouts but closing for now |
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Summary
This PR removes the need for an
Rcto implement interned by using theBufferas an internerStartInternedandEndInternedtag with the correspondingIDand write the content directly to the targetBuffer.Reference(id)node to reference to the interned content.The
Printerskips interned content but writes it to an internalIndex. ThePrinteruses the internalIndexto resolve theInternedcontent when it sees aReference. The fact thatids tend to be monotonic allows the use of aVecas anIndexsimilar toGroupidsThe downside of this approach is that it is more complicated to reason about and it is no longer possible to inspect the content of
Referencewithout tracking all interned content. Another downside is that the IR is slightly misleading because you seeInternedcontent and would assume it gets printed in that place, which is not true. For example, the interned content appears before thebest_fitting,but the content only gets printed inside the best-fitting variants. Moving theInternedcontent into the first best fitting variant doesn't work because that content only gets printed conditionally, meaning thePrinterwould then not always see theInternedcontent, failing to resolve theReference.Alternatives
One alternative that I explored is to use
bumpalloinstead and use it to allocateBestFittingvectors, dynamic strings, and Rcs. This could improve performance even more because it eliminates the 3-4% drop-time too. However, it comes at the downside thatBufferrequires a new'elementslifetime that needs to be added everywhere (Format,Formatter,Buffer,FormatNode,FormatNodeRule....). I gave up at some point because it made the API significantly more complicated.Test Plan
cargo test